feat(connectors): callApi for metered connectors - #256

Claude finished @ChenMachBase's task —— View job

Code Review: callApi for metered connectors

Thorough, well-tested PR. The two "shape decisions" (upstream errors resolve; query always forwarded) are correct and the tests pin both. Below is feedback organized by category.

Code quality — 👍

• proxyCall is cleanly factored out and documented; the @internal JSDoc explaining why upstream 4xx resolves rather than throws is exactly the kind of comment that earns its keep (src/modules/connectors.ts:139-147).
• New assertNonEmptyString helper reduces the repeated validation boilerplate. Nice.
• camelCase mapping and the raw envelope type (ConnectorProxyRawResponse) match the codebase convention of casting the interceptor's unwrapped response.data via as unknown as — consistent with the sibling methods.

Potential bugs / issues

1. method not validated / arbitrary strings pass through. request.method is typed as a union, but at runtime any string is accepted and uppercased (connectors.ts:159). The test even passes "post" lowercase. That's fine for the happy path, but an invalid method (e.g. "FETCH") is silently forwarded to the server. Minor — server will reject — but worth deciding if the SDK should guard.
2. body on GET/HEAD/DELETE. The type doc says body is "Ignored for GET, HEAD, DELETE," but the client always forwards body: request.body ?? null (connectors.ts:163). So the server must be the one ignoring it. That's a reasonable division, but the doc phrasing ("Ignored") reads as if the SDK drops it. Consider "Ignored by the server for GET/HEAD/DELETE" to match actual behavior.
3. Validation-order asymmetry. callApi validates integrationType before request is even checked for being an object; proxyCall then validates request and path. Given the test.each with ["x", ""] expects the path error, this ordering works, but the two-stage validation split across two functions is a little subtle. Not a bug — just flag for maintainers.

Performance — 👍

No concerns. Single POST per call; no extra allocations of note. The docs' warning about expensive calls inside loops is the right place to address the real cost lever, since it's server-priced.

Security

• The design (token never leaves the server for metered connectors) is sound and is the whole point.
• headers passthrough: the type doc says only connector-allowlisted headers are forwarded and the rest dropped server-side (connectors.types.ts:66). Good that the allowlist lives on the server rather than the client. No client-side secret handling here. ✅
• No logging of request bodies/headers added in this path (the iframe postMessage logging in axios-client.ts is pre-existing and unrelated).

Test coverage — strong

9 tests cover: normalization, GET default, query forwarding, camelCase mapping, upstream-error-resolves, Base44-error-rejects (402), metered 403 on getConnection, and empty identifier/path. A couple of gaps worth considering (optional):

• No test for missing/non-object request — proxyCall's "Request is required and must be an object" branch (connectors.ts:153-155) is uncovered. Easy to add: callApi("x", undefined as any).
• No test asserting status/headers/creditsCharged defaults when the server omits them (the ?? null / ?? {} / ?? 0 fallbacks at connectors.ts:169-172). A response missing those fields would exercise the nullish-coalescing paths.
• method uppercasing is implicitly tested via "post" → "POST", good.

Minor nits

• getConnection/getWorkspaceConnection/getCurrentAppUserConnection still use inline if (!x || typeof x !== "string") checks while callApi uses the new assertNonEmptyString. Not required for this PR, but a follow-up could unify them.
• ConnectorApiResponse.data is typed T but the proxy could return null/error bodies on success: false; callers relying on T without checking success may get a surprise. The docs cover this, so it's acceptable.

Overall: LGTM. The billing-bypass reasoning behind always forwarding query is well-justified, and error semantics are correct and tested. The only thing I'd actually add before merge is a test for the non-object request branch; everything else is optional polish.
• Branch: connectors-money

🚀 Package Preview Available!

Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.43-pr.256.e07d4b3

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.43-pr.256.e07d4b3"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.43-pr.256.e07d4b3"
  }
}

• 📦 Preview Package: @base44-preview/sdk@0.8.43-pr.256.e07d4b3
• 🔗 View this commit on GitHub

_{Preview published to npm registry — try new features instantly!}